Add HTTP1Connection #400
Add HTTP1Connection #400
Conversation
fabianfett
force-pushed
the
ff-http1-connection
branch
2 times, most recently
from
6b4c5cf to
068235c
Compare
Sources/AsyncHTTPClient/ConnectionPool/HTTP1.1/HTTP1ClientChannelHandler.swift
Show resolved
Hide resolved
| func close(context: ChannelHandlerContext, mode: CloseMode, promise: EventLoopPromise<Void>?) { | ||
| context.close(mode: mode, promise: promise) | ||
| } |
There was a problem hiding this comment.
Is this meant to just be the default implementation?
| self.logger.trace("New request to execute") | ||
|
|
||
| let req = self.unwrapOutboundIn(data) | ||
| self.request = req |
There was a problem hiding this comment.
What happens if there's already a request running?
There was a problem hiding this comment.
We will catch that in the state machine. But I guess it doesn't hurt to add another assert here!
|
|
||
| // MARK: - Run Actions | ||
|
|
||
| func run(_ action: HTTP1ConnectionStateMachine.Action, context: ChannelHandlerContext) { |
| switch action { | ||
| case .sendRequestHead(let head, startBody: let startBody): | ||
| if startBody { | ||
| context.write(wrapOutboundOut(.head(head)), promise: nil) |
There was a problem hiding this comment.
nit: explicit self for wrapOutboundOut (and below)
| } | ||
| } | ||
|
|
||
| private func clearIdleReadTimeoutTimer() { |
There was a problem hiding this comment.
Is this ever called? Looks like it was superseded by resetIdleReadTimeoutTimer.
There was a problem hiding this comment.
Added code that uses that :)
|
|
||
| private func writeRequestBodyPart0(_ data: IOData, request: HTTPExecutingRequest) { | ||
| guard self.request === request else { | ||
| // very likely we got threading issues here... |
There was a problem hiding this comment.
Who would have gotten this wrong, us or the caller? Should we assert/precondition accordingly?
| self.run(action, context: self.channelContext) | ||
| } | ||
|
|
||
| func cancelRequest0(_ request: HTTPExecutingRequest) { |
| final class HTTP1Connection { | ||
| let channel: Channel | ||
|
|
||
| /// the connection pool that created the connection |
There was a problem hiding this comment.
It's a delegate rather than a pool. Presumably a pool most of the time though.
| import NIOHTTP1 | ||
| import XCTest | ||
|
|
||
| class HTTP1ClientChannelHandlerTests: XCTestCase {} |
There was a problem hiding this comment.
I assume you'll add to this later.
fabianfett
force-pushed
the
ff-http1-connection
branch
from
ad97a25 to
c93cac7
Compare
fabianfett
force-pushed
the
ff-http1-connection
branch
from
c93cac7 to
663aaae
Compare
| } | ||
|
|
||
| func read(context: ChannelHandlerContext) { | ||
| self.logger.trace("Read") |
There was a problem hiding this comment.
Is there a plan to remove this log? If no, can we give it a better title?
| init(connection: HTTP1Connection, eventLoop: EventLoop, logger: Logger) { | ||
| self.connection = connection | ||
| self.eventLoop = eventLoop | ||
| self.logger = logger |
There was a problem hiding this comment.
Given the collection of basically anonymous logs emitted from this logger we absolutely must attach some metadata to it so we can filter them.
| // | ||
| // This source file is part of the AsyncHTTPClient open source project | ||
| // | ||
| // Copyright (c) 2018-2019 Apple Inc. and the AsyncHTTPClient project authors |
There was a problem hiding this comment.
Can you update the license field here?
fabianfett
force-pushed
the
ff-http1-connection
branch
from
7579b95 to
f7d2416
Compare
| private func start(configuration: HTTPClient.Configuration, logger: Logger) throws { | ||
| self.channel.eventLoop.assertInEventLoop() | ||
|
|
||
| guard case .initialized = self.state else { | ||
| preconditionFailure("Connection must be initialized, to start it") | ||
| } | ||
|
|
||
| self.state = .active | ||
| self.channel.closeFuture.whenComplete { _ in | ||
| self.state = .closed | ||
| self.delegate.http1ConnectionClosed(self) | ||
| } | ||
|
|
||
| do { | ||
| let sync = self.channel.pipeline.syncOperations | ||
| try sync.addHTTPClientHandlers() | ||
|
|
||
| if case .enabled(let limit) = configuration.decompression { | ||
| let decompressHandler = NIOHTTPResponseDecompressor(limit: limit) | ||
| try sync.addHandler(decompressHandler) | ||
| } | ||
|
|
||
| let channelHandler = HTTP1ClientChannelHandler( | ||
| connection: self, | ||
| eventLoop: channel.eventLoop, | ||
| logger: logger | ||
| ) | ||
|
|
||
| try sync.addHandler(channelHandler) | ||
| } catch { | ||
| self.channel.close(mode: .all, promise: nil) | ||
| throw error | ||
| } |
There was a problem hiding this comment.
@Lukasa dragged the stuff out!
This PR adds an
HTTP1Connectionclass that will be used in our newConnectionPool.This PR is under-tested as of right now, but I would love to get some early feedback on the current implementation.